[18.0][POC] base_exception: Rollback transaction triggering exception rule #3476
+72
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case a implementation module, eg sale_exception, breaks a function override, ie does not call super on sale.order.action_confirm in case an exception rule applies, it is still possible that another module that is not in the implementation module's dependency, does modify existing the same object through the same function's override that is called before through MRO. As the exception rule is only written in base_exception module, such modifications would be still be committed although the exception was triggered. The only way to rollback everything that happened is to raise an exception that is going to rollback the transaction. However, we still want to commit the write of the exception rule and not to propagate the exception back to the webclient. Since we cannot alter the env in the retrying function (that handles rollback), we do not have any other choice than to use a dedicated cursor in the dispatching function to process the request, since it could trigger the exception rule, and to use the original env to write the exception rule, and have the webclient being refreshed to display the exception. This might break the latest feature to pop up the wizard, but since this is still a POC, we could fix that afterwards in case this POC is accepted.
Contributor
|
Hi @sebastienbeau, @hparfr, |
SilvioC2C
reviewed
Dec 27, 2025
base_exception/models/ir_http.py
Outdated
Comment on lines
34
to
37
| for rule_id, (model, res_ids) in to_add.items(): | ||
| old_env[model].browse(res_ids).write({"exception_ids": [(4, rule_id)]}) | ||
| for rule_id, (model, res_ids) in to_remove.items(): | ||
| old_env[model].browse(res_ids).write({"exception_ids": [(4, rule_id)]}) |
Contributor
There was a problem hiding this comment.
Beware:
- in the old behavior, old rules were removed before new ones got added, while in here you do the opposite: is this a wanted change?
- the command value should be
3when removing rules,4when adding them, while in here you're always using4: can you please switch toCommand.[un]link(ID)instead to make it easier to read and debug (and also, possibly add tests)?
Contributor
Author
There was a problem hiding this comment.
Good catch, I did focus on the transaction handling, but both are valid points 👍
Contributor
|
So you want to use an Why not |
Contributor
Author
|
@hparfr I'm not sure if you understood the issue here 😕 This is actually a potential fix to all of these issues:
It happened to some of our OE customers using module sale_subscription and sale_exception , when such conditions are verified:
Linked to this, #3405 is actually a workaround to avoid applying the exception for records (eg upsell orders in the example I just provided). |
Contributor
No issue at all from my side. I find it "fun" to have have a true I read the code, but did not test it yet. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In case a implementation module, eg sale_exception, breaks a function override, ie does not call super on sale.order.action_confirm in case an exception rule applies, it is still possible that another module that is not in the implementation module's dependency, does modify existing the same object through the same function's override that is called before through MRO.
As the exception rule is only written in base_exception module, such modifications would be still be committed although the exception was triggered.
The only way to rollback everything that happened is to raise an exception that is going to rollback the transaction. However, we still want to commit the write of the exception rule and not to propagate the exception back to the webclient.
Since we cannot alter the env in the retrying function (that handles rollback), we do not have any other choice than to use a dedicated cursor in the dispatching function to process the request, since it could trigger the exception rule, and to use the original env to write the exception rule, and have the webclient being refreshed to display the exception.
This might break the latest feature to pop up the wizard, but since this is still a POC, we could fix that afterwards in case this POC is accepted.